Repository navigation
Simplify an opened multiple angle before judging it (#557) - #732
Merged
Merged
Conversation
The reporter's second expression is 0 and came back as a sum of five terms. Its first was already handled -- opening sin(2t) up is what settles it -- and the second needs the same opening to survive one step further than it did. Opening a multiple angle is offered as a candidate rather than taken, because written out sin(4x) is longer than it started, and the complexity metric decides. That judgement is right; what was wrong is the form it was asked to judge. The opened form was handed to the trigonometric rules only, which settles an expression that is already a single term. This one is a quotient, and it cancels only once its terms are over a common denominator -- and the passes that build one run earlier in the loop, while the angles are still shut. So the metric saw the opened form with none of its payoff collected and rightly rejected it as the longer of the two. It is now simplified in full first, which is what Expand and Factorize already get two lines above, and for the same reason. Expanded as well: the cancellation here only appears once the products are multiplied out, so `sin(t)^6 - (2 sin(t) cos(t))^2 sin(t)^4/4` over `sin(t)^8` has to be opened before it reads as 1. Only one candidate, not two. Offering the unexpanded opening as well made no difference to any expression measured and doubled the added cost. Measured: the reporter's second expression from a sum of five terms to `0 provided not sin(t) = 0`, in 269 ms. Multiple angles that do not cancel keep their compact form -- sin(4x), cos(6x) and sin(3x) are unchanged, and sin(8x)cos(8x) still collapses to sin(16x)/2 -- and cost roughly 1.2 to 2 times what they did, in the tens of milliseconds; only an expression that contains a multiple angle pays anything. Full suite 4858 passed / 0 failed, F# 130/130, corpus 112/117 with 0 wrong, 0 error, 0 timeout, and unchanged in total time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #557.
The reporter gives two expressions. Both are 0. The first was already handled — opening
sin(2t)up is what settles it — and the second came back as a sum of five terms.What was wrong
Opening a multiple angle is offered as a candidate rather than taken, because written out
sin(4x)is longer than it started and the complexity metric should decide. That judgement is right. What was wrong is the form it was asked to judge.The opened form was handed to the trigonometric rules only, which settles an expression that is already a single term. The reporter’s second expression is a quotient, and it cancels only once its terms are over a common denominator — and the passes that build one run earlier in the loop, while the angles are still shut. So the metric saw the opened form with none of its payoff collected, and rightly rejected it as the longer of the two.
That the cancellation is real is easy to see in isolation:
Same expression, and only the shut form failed.
The fix
The opened form is simplified in full before being offered — which is exactly what
Expand()andFactorize()already get two lines above, and for the same reason. Expanded as well, since the cancellation here only appears once the products are multiplied out.One candidate, not two: offering the unexpanded opening alongside it made no difference to any expression measured and doubled the added cost.
Measured
0 provided not sin(t) = 0(269 ms)Multiple angles that do not cancel keep their compact form —
sin(4x),cos(6x)andsin(3x)are unchanged, andsin(2x)*cos(2x)still collapses tosin(4x)/2. They cost roughly 1.2–2× what they did, in the tens of milliseconds, and only an expression that contains a multiple angle pays anything at all.Full suite 4858 passed / 0 failed, F# 130/130, corpus 112/117 with 0 wrong, 0 error, 0 timeout and unchanged in total time.
🤖 Generated with Claude Code